agentling/feature/memory#3
Conversation
folathecoder
commented
Jul 6, 2026
- Add memory.py: typed steps, Memory, and JSON round-trip
- Add events.py: frozen streaming event types (TextDelta, ToolCall/Result, Step, Final)
- Add unit tests for memory.py (message shapes, error rendering, JSON round-trip)
Record each turn as a typed Step (TaskStep/ActionStep/FinalStep) that renders itself to ChatMessages, rather than storing a flat message list. Steps carry usage/timing/errors; failed tools render as recoverable observations. Memory derives the model's message list via to_messages(system_prompt), and dump_json/load_json round-trip the whole run (versioned) for persistence/replay.
PR Summary by QodoAdd typed run Memory with step records, streaming Events, and JSON persistence
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Fragile JSON load path
|
| @classmethod | ||
| def from_dict(cls, data: dict[str, Any]) -> Memory: | ||
| """Rebuild a Memory from the dict produced by to_dict().""" | ||
|
|
||
| return cls(steps=[_step_from_dict(entry) for entry in data.get("steps", [])]) | ||
|
|
||
| def dump_json(self) -> str: | ||
| """Serialize the memory to a JSON string.""" | ||
|
|
||
| return json.dumps(self.to_dict()) | ||
|
|
||
| @classmethod | ||
| def load_json(cls, raw: str) -> Memory: | ||
| """Rebuild a Memory from a JSON string produced by dump_json().""" | ||
|
|
||
| return cls.from_dict(json.loads(raw)) |
There was a problem hiding this comment.
1. Fragile json load path 🐞 Bug ☼ Reliability
Memory.from_dict() iterates over data.get('steps', []) without normalizing/validating, so inputs
like {"steps": null} raise TypeError and malformed entries raise KeyError deep in helpers instead of
a clear ValueError. The serializer emits a version field but the loader ignores it, making format
evolution harder and failures less diagnosable.
Agent Prompt
### Issue description
`Memory.from_dict()` / `_step_from_dict()` / `_chat_message_from_dict()` assume a perfectly-shaped dict matching `to_dict()` output. If `steps` is present but `null` (or not a list), or if nested keys are missing (e.g. `tool_calls`, `tool_call_id`, `tool_results`, `error`, `duration`), loading raises low-level `TypeError`/`KeyError` rather than a controlled `ValueError`. Also, `to_dict()` writes a `version` field but `from_dict()` never checks it.
### Issue Context
This code is intended for persistence/replay. In practice, stored JSON can be manually edited, partially migrated, or come from a newer/older schema version.
### Fix Focus Areas
- src/agentling/memory.py[152-168]
- src/agentling/memory.py[172-200]
### What to change
- In `from_dict`, normalize `steps = data.get("steps") or []` and validate it’s a list of dict entries; raise `ValueError` with a helpful message if invalid.
- Read and validate `version` (e.g. default to 1; raise on unsupported versions).
- In `_chat_message_from_dict` and `_step_from_dict`, use `.get()` with sensible defaults for optional fields (`tool_calls`, `tool_call_id`, `usage`, `tool_results`, `error`, `duration`) and raise `ValueError` on missing required fields (`role`, `content`, etc.) rather than allowing `KeyError`.
- Optionally, accept missing optional keys for backward compatibility (e.g. treat missing `tool_calls` as `[]`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| return { | ||
| "version": 1, | ||
| "steps": [ | ||
| {"type": self._KIND[type(step)], "data": asdict(step)} | ||
| for step in self.steps | ||
| ], |
There was a problem hiding this comment.
2. Subclass steps break serialization 🐞 Bug ⚙ Maintainability
Memory.to_dict() uses self._KIND[type(step)] for tagging, so any subclass of TaskStep/ActionStep/FinalStep will raise KeyError and prevent persistence. This makes the step system unexpectedly non-extensible at runtime.
Agent Prompt
### Issue description
`Memory.to_dict()` tags steps via `self._KIND[type(step)]`, which only works for exact built-in step classes. Subclasses (even compatible ones) won’t be found and will crash serialization with `KeyError`.
### Issue Context
Even if the library doesn’t officially support custom step subclasses today, this behavior is surprising and easy to trip over when users add metadata via subclassing.
### Fix Focus Areas
- src/agentling/memory.py[105-150]
### What to change
- Replace exact-type lookup with a more robust discriminator:
- Add a `kind: ClassVar[str]` on each step class, or
- Resolve kind via `next(tag for cls, tag in _KIND.items() if isinstance(step, cls))`, raising a clear `ValueError` if no match.
- Consider adding a small unit test that verifies subclass instances either serialize correctly or fail with a clear error message.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools